Fix js sync on animation end in Animation Backend#55566
Open
bartlomiejbloniarz wants to merge 1 commit intomainfrom
Open
Fix js sync on animation end in Animation Backend#55566bartlomiejbloniarz wants to merge 1 commit intomainfrom
bartlomiejbloniarz wants to merge 1 commit intomainfrom
Conversation
meta-cla
bot
added
the
CLA Signed
|
@bartlomiejbloniarz has exported this pull request. If you are a Meta employee, you can view the originating Diff in D93414839. |
Summary: In Animation Backend, we allow animation frameowrks to request a js-thread sync of the current animation state. It is used by c++ Animated, and is meant to serve as a way to push animation changes to react through RSNRU, after the animation finsishes. This way we ensure that subsequent rerenders of the component don't bring back the old style value. This approach is currently broken when the animation performs any main-thread commits, as in this case the `runtimeShadowNodeReference_` is not copied to new node revisions, so the js-thread sync commit cannot use RSNRU properly. The bug was not visible, because we don't clean up the registry in that case, we only do it for react commits. This PR fixes the issue for the case when `updateRuntimeShadowNodeReferencesOnCommitThread` is enabled, as this fixes the RSNRU propagation, so we can clean-up the registry. If the flag is disabled, we don't cleanup the registry, as we want the next react commit to make sure the animation state is not overwritten. # Changelog [General][Added] - test for the Animation Backend js sync [General][Changed] - TesterAnimationChoreographer changes the thread_local RSNRU flag when running animation update, to better simulate the real application use-case [General][Changed] - AnimationBackend now cleans-up the AnimatedPropsRegistry after the js sync when `updateRuntimeShadowNodeReferencesOnCommitThread` is enabled Differential Revision: D93414839
facebook-github-bot
force-pushed
the
export-D93414839
branch
from
dd78ff9 to
29fb8d8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
In Animation Backend, we allow animation frameowrks to request a js-thread sync of the current animation state. It is used by c++ Animated, and is meant to serve as a way to push animation changes to react through RSNRU, after the animation finsishes. This way we ensure that subsequent rerenders of the component don't bring back the old style value.
This approach is currently broken when the animation performs any main-thread commits, as in this case the
runtimeShadowNodeReference_is not copied to new node revisions, so the js-thread sync commit cannot use RSNRU properly. The bug was not visible, because we don't clean up the registry in that case, we only do it for react commits.This PR fixes the issue for the case when
updateRuntimeShadowNodeReferencesOnCommitThreadis enabled, as this fixes the RSNRU propagation, so we can clean-up the registry. If the flag is disabled, we don't cleanup the registry, as we want the next react commit to make sure the animation state is not overwritten.Changelog
[General][Added] - test for the Animation Backend js sync
[General][Changed] - TesterAnimationChoreographer changes the thread_local RSNRU flag when running animation update, to better simulate the real application use-case
[General][Changed] - AnimationBackend now cleans-up the AnimatedPropsRegistry after the js sync when
updateRuntimeShadowNodeReferencesOnCommitThreadis enabledDifferential Revision: D93414839